After regenerate_public_key_encryption_key_pair_if_needed_with_ciphers(&sync.ciphers) runs — and
potentially succeeds in posting new keys to the server — the returned
Option<WrappedAccountCryptographicState> is discarded by the ? operator. The original sync
value is then forwarded unchanged into internal_rotate_user_keys, whose
wrapped_account_cryptographic_state field still holds the pre-regeneration wrapped private key
EncString.

Downstream, rotate_account_cryptographic_state_to_wrapped_model calls
WrappedAccountCryptographicState::rotate(...) →
ctx.unwrap_private_key(current_user_key, private_key) on that stale EncString. Regeneration is
only triggered via the NeedsCipherCheck path — which fires precisely when the old EncString
cannot be decrypted. So the rotation call fails with RotateCryptographyStateError::InvalidData
immediately after the server has committed the new key pair.

Net effect: the PR's stated goal — "ensures that key rotation can proceed even if the existing
private key is corrupt" — is not achieved on the first call. A retry works (sync now returns
the regenerated, decryptable key), but the user receives an error mid-flow after server state was
already mutated.

The docstring on regenerate_public_key_encryption_key_pair_if_needed explicitly states: "Callers
should persist the returned state to their local account cryptographic state." — the contract is
violated by both new callers.

Implicates EK (encryption key integrity) and P06 (minimized impact of breaches —
half-completed rotation after regeneration worsens recovery).

Fix: When regenerate_public_key_encryption_key_pair_if_needed_with_ciphers returns
Some(new_state), replace sync.wrapped_account_cryptographic_state with new_state before
calling internal_rotate_user_keys:

let new_state = self.regenerate_public_key_encryption_key_pair_if_needed_with_ciphers(&sync.ciphers)
    .await
    .map_err(|_| RotateUserKeysError::CryptoError)?;
if let Some(state) = new_state {
    sync.wrapped_account_cryptographic_state = state;
}
internal_rotate_user_keys(key_store, api_client, request, sync).await

Apply the same fix to password_change_and_rotate_user_keys.rs.

Mirror of the rotate_user_keys.rs defect.
regenerate_public_key_encryption_key_pair_if_needed_with_ciphers is called and its
Option<WrappedAccountCryptographicState> return is discarded.
internal_password_change_and_rotate_user_keys is then invoked with the original sync whose
wrapped_account_cryptographic_state is still the pre-regeneration value.

WrappedAccountCryptographicState::rotate will fail to unwrap_private_key on the undecryptable
EncString that triggered regeneration in the first place — after
POST /accounts/key-management/regenerate-keys has already committed to the server. The
password-change-and-rotate call fails with RotateUserKeysError::Crypto for exactly the class of
users this PR is designed to help.

Fix: Same pattern as the rotate_user_keys fix — consume the returned
Option<WrappedAccountCryptographicState> and overwrite sync.wrapped_account_cryptographic_state
if Some.

check_key_pair fetches private_key from the server's GET /accounts/keys response and tries to
unwrap it with the user's local key. If the server returns a syntactically valid EncString that
the user key cannot decrypt, flow falls through to KeyPairCheckResult::NeedsCipherCheck. If any
personal cipher then decrypts successfully (user key is valid), the function returns Ok(true) —
triggering a full key regeneration via POST /accounts/key-management/regenerate-keys.

This means a hostile or compromised server can force key regeneration at will by returning
garbage in the private_key field while leaving public_key valid. The regeneration replaces the
user's actual valid private key on the server, permanently destroying access to any items encrypted
with the old public key: org-shared items, Sends, and emergency-access designations.

Implicates P05 (controlled access — regeneration is triggered solely by server-controlled data,
with no user prompt) and P06 (minimized impact of breaches — a transient server compromise can
permanently destroy shared-data accessibility).

The code does verify whether the server's claimed private key decrypts and matches the server's
claimed public key, but it does not check whether the local keystore's existing UserPrivateKey
slot already holds a valid key that matches the server's public_key. If it does, the server's
private_key claim is untrustworthy and regeneration should be refused.

Suggested mitigation: Before falling through to NeedsCipherCheck, check whether the existing
UserPrivateKey slot in the local keystore is present and its derived public key matches the
server's public_key. If it does, skip regeneration regardless of whether the server's
private_key EncString is decryptable.

internal_regenerate_public_key_encryption_key_pair has a two-phase structure:

1. Generate new key pair, wrap it, get wrapped bytes and public key (first scope).
2. POST /accounts/key-management/regenerate-keys — server commits the new key pair.
3. Re-enter keystore, unwrap_private_key and persist_private_key to
   PrivateKeySlotId::UserPrivateKey (second scope).

If step 3 fails (CryptoError), the function returns Err(KeyPairRegenerationError::CryptoError) —
but the server has already committed the new key pair. The client's keystore retains the old
UserPrivateKey (or none). No rollback is attempted; there is no distinct error variant to signal a
desync state to the caller.

The caller maps CryptoError to RotateUserKeysError::CryptoError — a generic failure with no
signal to re-login and re-sync.

Implicates EK durability (locally cached private key material becomes inconsistent with
server-side state) and P05 (user has no visibility into or control over the resulting desynced
state).

Suggested mitigation: Persist the new private key to a temporary slot in the keystore before
the API call, swap it to UserPrivateKey only after the API call confirms, and on post-API failure
return a distinct error variant (e.g., RegenerateCommittedButLocalPersistFailed) so callers can
force a re-sync rather than treating it as a generic crypto error.

is_user_key_valid_from_api and is_user_key_valid both return Ok(false) (skip regeneration)
when:

• No personal ciphers exist (only org-owned ciphers)
• The cipher fetch API call fails
• Cipher parsing fails

A fresh user (no vault items yet), or a user whose vault contains only organization-shared items,
can reach the NeedsCipherCheck path (undecryptable private key + no personal ciphers) but the gate
never opens — should_regenerate returns false, the callers proceed as-if-fine, and downstream
rotation fails when it tries to unwrap the corrupt private key.

Ok(false) is indistinguishable from "no regeneration needed" to callers, so the rotation proceeds
and produces a RotateUserKeysError::Crypto with no explanation. These users cannot recover via
this PR's fix.

Implicates VD availability (vault data inaccessible due to corrupt key pair cannot be
remediated) and the PR's stated correctness goal.

Suggested mitigations:

1. Validate the user key against an artifact that doesn't depend on cipher count — e.g., decrypt the
   master-password-protected user key envelope (if available in sync data) to confirm the user key
   is correct.
2. Surface a distinct return variant (e.g.,
   Err(KeyPairRegenerationError::UnableToValidateUserKey)) so the caller can distinguish "no
   regeneration needed" from "skipped due to no validation artifact," and handle the latter
   explicitly.

regenerate_public_key_encryption_key_pair_if_needed is exposed as a public method and registered
via #[cfg_attr(feature = "wasm", wasm_bindgen)]. It calls
POST /accounts/key-management/regenerate-keys whenever the internal heuristic votes yes — with no
user-visible prompt, confirmation parameter, or rate limit.

Key regeneration is an irreversible, destructive operation: it permanently replaces the user's
public/private key pair on the server, invalidating access to all items encrypted with the old
public key (org-shared ciphers, Sends, emergency-access designations). Because the heuristic is
driven by server-controlled data (see sec-3), a hostile server can cause this public API to trigger
unwanted regeneration from any process that calls it on an unlocked client.

Implicates P05 (vault data access must be under the user's explicit control) and P06
(minimized impact of breaches — an automatically invoked destructive action worsens the blast radius
of a server compromise).

Suggested mitigation options:

1. Add an explicit user_consented: bool parameter (or a typed consent token) that callers must
   pass; enforce it in the method body.
2. Restrict the public surface to should_regenerate_public_key_encryption_key_pair only. Require
   the caller (UI layer) to present a destructive-action confirmation dialog, then invoke
   regeneration through a separate explicit method.

The PR moved the KeyConnector | Tde → UnimplementedKeyRotationMethod guard from
internal_rotate_user_keys (easily unit-tested in isolation) to the public rotate_user_keys
method. The two corresponding tests (test_rotate_user_keys_key_connector_returns_unimplemented,
test_rotate_user_keys_tde_returns_unimplemented) were removed without replacement.

The guard is now untested. When KeyConnector or Tde rotation is eventually implemented and the
guard comment ("This guard should be removed once other key rotation methods are implemented") is
acted on, there is no safety net to catch an accidental removal or mis-scoping.

Fix: Add tests against the public rotate_user_keys (via UserCryptoManagementClient) that
assert KeyRotationMethod::KeyConnector and KeyRotationMethod::Tde return
Err(RotateUserKeysError::UnimplementedKeyRotationMethod) without making API calls.

The new is_v1_symmetric_key method on KeyStoreContext introduces violations of the crypto
crate's documented conventions:

1. README rule. crates/bitwarden-crypto/README.md states: "Limit public interfaces to the
   bare minimum." The method is a thin wrapper over the already-public
   get_symmetric_key_algorithm. All callers in this PR could use
   ctx.get_symmetric_key_algorithm(id)? == SymmetricKeyAlgorithm::Aes256CbcHmac directly with no
   loss in clarity.

2. Vocabulary leak across architectural layers. "V1" and "V2" are account cryptographic state
   concepts defined in bitwarden-core::key_management::account_cryptographic_state. The crypto
   crate has no other reference to V1/V2 — it deals in algorithm names (Aes256CbcHmac,
   XChaCha20Poly1305). Embedding is_v1_* here couples a foundation-layer crate to a
   feature-layer concept and invites future is_v2_* / is_v3_* proliferation. Note that V1/V2 is
   not purely an algorithm property — it is a property of the account's full key set.

3. Naming convention inconsistency. Existing public methods on KeyStoreContext uniformly use
   get_*, has_*, make_*, wrap_*, unwrap_*, set_*, add_*. There are no existing is_*
   methods.

Fix: Remove is_v1_symmetric_key. Replace call sites with
get_symmetric_key_algorithm(id)? == SymmetricKeyAlgorithm::Aes256CbcHmac. If a named helper is
wanted, define it as a private free function inside bitwarden-core::key_management or
bitwarden-user-crypto-management, where the V1/V2 vocabulary belongs.

is_user_key_valid_from_api and is_user_key_valid are documented as: "Returns Ok(true) (should
regenerate) if the user key is valid, Ok(false) otherwise."

The function names assert is_valid? semantics (a property of the key), but the boolean carries
should_regenerate? semantics (a decision). A future reader following the function name can easily
invert the boolean, or mistake the return value for a general key-validity predicate usable outside
this regeneration decision tree.

Fix: Rename to should_regenerate_from_api / should_regenerate_from_ciphers (making the
decision semantics explicit at every call site), or make the functions return the literal is the
user key valid? boolean and let check_key_pair decide whether to regenerate.

Discarded regeneration result — stale wrapped private key in rotate_user_keys (quality)

crates/bitwarden-user-crypto-management/src/key_rotation/rotate_user_keys.rs:70-75 Caught by:
Code quality agent Original severity: 🛑 Blocker Original confidence: 90/100 Dismissed
at: Step 4 validation Dismissed because: Duplicate of sec-1 — identical root defect (discarded
regeneration result, stale sync.wrapped_account_cryptographic_state forwarded to
internal_rotate_user_keys); sec-1 was retained as it articulates both failure modes.

Discarded regeneration result — stale wrapped private key in password_change_and_rotate (quality)

crates/bitwarden-user-crypto-management/src/key_rotation/password_change_and_rotate_user_keys.rs:54-59
Caught by: Code quality agent Original severity: 🛑 Blocker Original confidence: 90/100
Dismissed at: Step 4 validation Dismissed because: Duplicate of sec-2 — identical root
defect in password_change_and_rotate_user_keys.rs; sec-2 was retained.

Stale sync data in password_change_and_rotate (bug analysis)

crates/bitwarden-user-crypto-management/src/key_rotation/password_change_and_rotate_user_keys.rs:60
Caught by: Bug analysis agent Original severity: 🛑 Blocker Original confidence: 90/100
Dismissed at: Step 4 validation Dismissed because: Duplicate of sec-2 — same root defect;
sec-2 was retained.

Stale sync data in rotate_user_keys (bug analysis)

crates/bitwarden-user-crypto-management/src/key_rotation/rotate_user_keys.rs:76 Caught by: Bug
analysis agent Original severity: 🛑 Blocker Original confidence: 90/100 Dismissed at:
Step 4 validation Dismissed because: Duplicate of sec-1 — same root defect; sec-1 was retained.

In the new rotate_user_keys public method, sync_current_account_data is called once, then
regenerate_public_key_encryption_key_pair_if_needed_with_ciphers is called with &sync.ciphers,
and finally the same unmodified sync is passed to internal_rotate_user_keys. If regeneration
actually occurred — the intended recovery path for a corrupt or missing private key —
sync.wrapped_account_cryptographic_state still contains the old encrypted private key fetched
from the server before regeneration. The return value of regenerate_...if_needed_with_ciphers
(Option<WrappedAccountCryptographicState>) is discarded via .map_err(...)?. When
internal_rotate_user_keys then calls
rotate_account_cryptographic_state_to_wrapped_model(&sync.wrapped_account_cryptographic_state, ...),
the decryption of the old corrupt private key fails, returning RotateUserKeysError::Crypto. The
regeneration succeeded (new key posted to server, key store slot updated), so the rotation fails on
precisely the user state this PR is designed to fix.

WrappedAccountCryptographicState::get_v1_from_key_store was added in this same PR and is the
correct tool for re-deriving the state from the key store after regeneration — but it is not called
here.

Implicates EK and P06 (minimized impact of security breaches: the recovery mechanism is
silently broken).

Fix: Capture the return value of regenerate_...if_needed_with_ciphers. If Some(new_state) is
returned, set sync.wrapped_account_cryptographic_state = new_state before passing sync to the
internal function:

if let Some(new_state) = self
    .regenerate_public_key_encryption_key_pair_if_needed_with_ciphers(&sync.ciphers)
    .await
    .map_err(|_| RotateUserKeysError::CryptoError)?
{
    sync.wrapped_account_cryptographic_state = new_state;
}

Identical root cause to the previous finding. In the new password_change_and_rotate_user_keys
public method, sync is fetched once, regeneration is called (return value discarded), then the
stale sync is passed to internal_password_change_and_rotate_user_keys. The internal function
calls
rotate_account_cryptographic_state_to_request_model(&sync.wrapped_account_cryptographic_state, ...),
which attempts to decrypt the old (potentially corrupt) private key from sync. This fails, leaving
users who needed regeneration unable to change their password and rotate keys — the exact scenario
this PR targets.

Implicates EK and P06.

Fix: Same as above — propagate Some(new_state) from regeneration into
sync.wrapped_account_cryptographic_state before calling the internal function.

The diff removes two pub fn methods from CryptoClient — make_key_pair and
verify_asymmetric_keys — both compiled into the WASM bindings via the
#[cfg_attr(feature = "wasm", wasm_bindgen)] on the impl block. CLAUDE.md states explicitly:

"Breaking changes: Automated detection via cross-repo workflow. TypeScript compilation tested
against clients repo on PR. Document migration path for clients."

The diff contains no CHANGELOG entry, no deprecation notice prior to removal, and no documentation
of what callers should use instead. The automated cross-repo workflow will catch compilation
failures in clients, but the project rule requires documenting the migration path independently of
whether CI blocks.

Fix: Add a CHANGELOG entry noting these methods were removed as redundant (key pair management
is now handled via regenerate_public_key_encryption_key_pair_if_needed), and document the
migration path for any callers.

The check_key_pair function determines whether to regenerate the user's RSA key pair entirely from
data returned by the server (GET /accounts/keys). A malicious or compromised server can trigger
silent key regeneration by returning any of:

• (None, None) for public/private keys
• A partial key pair (one present, one absent)
• An unparseable EncString for the private key
• A public key string that doesn't match the real one on the server

In each case the client silently replaces the user's RSA key pair without user consent, warning, or
fallback check against a locally cached key pair state.

This violates P01 (Servers are Zero Knowledge): the server must not be able to force replacement
of client-side key material. Replacing the RSA key pair invalidates all data previously encrypted to
the user's public key — specifically org-shared vault items, emergency access grants, and trusted
device keys. A compromised server could force every unlocked V1 user to replace their RSA key pair
on the next rotation attempt, breaking all org-shared data access silently.

Implicates EK: encryption key regeneration must not be triggerable by an untrusted party.

Suggested mitigations:

1. Before regenerating based on (None, None), compare against the locally stored
   WrappedAccountCryptographicState.private_key — regenerate only if local state is also absent or
   locally verifiable as corrupt.
2. If no local state is available to cross-reference, require explicit user acknowledgement before
   replacing a key pair.
3. At minimum, treat a (None, None) response from the server skeptically when the user has
   previously been initialized with a key pair.

In both password_change_and_rotate_user_keys and rotate_user_keys,
regenerate_public_key_encryption_key_pair_if_needed_with_ciphers is called silently before
rotation proceeds. If regeneration fires, the user's RSA key pair is replaced without any signal to
the user — no warning, no prompt, no indication in the rotation functions' return values or error
paths.

Replacing the RSA key pair has significant downstream consequences: all existing org-shared vault
items (re-encrypted to the old public key by org admins), emergency access grants, and trusted
device authorizations that reference the old public key are now effectively invalidated. The PR
description acknowledges a coordinated server-side PR must "handle status changes for emergency
access/organizations" — meaning the client silently performs an action whose full consequences
depend on a server-side deployment.

Under P05 (Controlled Access to Vault Data): vault data must be accessible only under the user's
explicit control. Silently replacing the key that org admins used to share data removes the user's
awareness and control over when that action occurs.

Fix: The public regenerate_public_key_encryption_key_pair_if_needed already returns
Option<WrappedAccountCryptographicState> — Some(state) when regeneration was performed. The
rotation callers should propagate this signal to the application layer, so client applications can
notify users that their RSA key pair was regenerated and that org/emergency-access relationships may
need re-establishment.

The regenerate_public_key_encryption_key_pair_if_needed_with_ciphers method's docstring inherits
the contract "Callers should persist the returned state to their local account cryptographic state."
However, both call sites in rotate_user_keys.rs and password_change_and_rotate_user_keys.rs drop
the returned Option<WrappedAccountCryptographicState> entirely — intentionally, because
internal_regenerate_public_key_encryption_key_pair already calls persist_private_key to update
the key store in-place, and the subsequent rotation will produce a fresh wrapped state.

This creates a misleading contract for future callers of the pub(crate) method.

Fix: Clarify the docstring to distinguish contexts: the key store is always updated in-place;
the returned Option<WrappedAccountCryptographicState> only needs to be persisted to the account's
durable storage when the caller will not subsequently rotate keys (i.e., the standalone
regenerate_public_key_encryption_key_pair_if_needed path).

In is_user_key_valid, when the private key cannot be decrypted and no personal ciphers exist (user
has only org ciphers), the function returns Ok(false) — "do not regenerate." The function cannot
distinguish between "user key is wrong" and "private key is corrupt" without a personal cipher to
test against. The conservative behavior is a warn! + skip.

Consequence: users with a corrupt private key and no personal ciphers will silently proceed into a
failed key rotation with a cryptographic error and no actionable diagnostic. They cannot resolve the
corrupt private key via this flow.

The PR's stated goal is to "fix corrupt private keys before key rotation." Silently skipping
regeneration for this user subset undermines that goal without surfacing any information to the
caller about why.

Fix: Return a distinct error variant (e.g., KeyPairRegenerationError::CannotValidateUserKey)
when the private key is undecryptable and no validation cipher is available. Callers can then
surface a meaningful diagnostic to the user ("your private key appears corrupt but we cannot safely
regenerate it because no personal vault items are available to validate the user key").

make_v1 promoted from #[cfg(test)] to pub(crate) — test factory leaked into production compilation

crates/bitwarden-core/src/key_management/account_cryptographic_state.rs:353 Caught by:
Architecture agent Original severity: ♻️ Refactor Original confidence: 83/100 Dismissed
at: Step 4 validation Dismissed because: The #[cfg(test)] attribute is preserved in the diff
— the change is from fn make_v1 to pub(crate) fn make_v1 while #[cfg(test)] remains on the
function. The method is still only compiled in test builds and does not enter production-compiled
code. The finding's central claim was factually incorrect.

is_v1_symmetric_key added to KeyStoreContext breaks inspection pattern

crates/bitwarden-crypto/src/store/context.rs:400 Caught by: Architecture agent Original
severity: ♻️ Refactor Original confidence: 80/100 Dismissed at: Step 4 validation
Dismissed because: The method is used at four or more call sites across
account_cryptographic_state.rs, regenerate.rs, and should_regenerate.rs. It encapsulates the
V1-specific algorithm comparison in a clearly named predicate, reducing coupling to the internal
SymmetricKeyAlgorithm::Aes256CbcHmac enum variant. A senior engineer would not flag a well-used
helper as refactor-worthy tech debt.

Tests for unimplemented KeyConnector/TDE rotation deleted without replacement

crates/bitwarden-user-crypto-management/src/key_rotation/rotate_user_keys.rs:57 Caught by:
Code quality agent Original severity: ⚠️ Important Original confidence: 90/100 Dismissed
at: Step 5 severity audit Dismissed because:
test_rotate_user_keys_key_connector_returns_unimplemented and
test_rotate_user_keys_tde_returns_unimplemented are present in the post-PR file at lines 272–331,
still calling internal_rotate_user_keys where the guard for these methods remains. No coverage gap
exists.

Sync API failure test deleted after moving sync call to public method

crates/bitwarden-user-crypto-management/src/key_rotation/rotate_user_keys.rs:80 Caught by:
Code quality agent Original severity: ⚠️ Important Original confidence: 85/100 Dismissed
at: Step 5 severity audit Dismissed because: Both tests are present in the post-PR files.
test_rotate_user_keys_api_failure_returns_api_error exists at lines 333–365 in
rotate_user_keys.rs, and
test_password_change_and_rotate_user_keys_sync_api_failure_returns_api_error exists at lines
255–286 in password_change_and_rotate_user_keys.rs. The diff removed old test implementations and
new equivalents were added as part of the refactor.